Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Decode mod_cms content on restore #125

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Decode mod_cms content on restore #125

merged 2 commits into from
Aug 28, 2024

Conversation

Fragonite
Copy link
Contributor

@Fragonite Fragonite commented May 23, 2024 •
edited
Loading

Resolves two problems.

  • Fixes customfield content not being decoded after restore. See also MDL-81913.
  • Fixes a problem where the instance cache key was based on the pre-decoded content, resulting in multiple unique instances sharing the same cached value.

Note that instances with identical content may still share the same cached value.

php vendor/bin/phpunit --testsuite mod_cms_testsuite
Moodle 4.1.9+ (Build: 20240215), ded168b0066144a8223615de5088b3cc65a527fb
Php: 8.0.30, pgsql: 14.0 (Debian 14.0-1.pgdg110+1), OS: Linux 6.5.0-35-generic x86_64
PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

............................................................      60 / 60 (100%)

Time: 00:33.128, Memory: 129.00 MB

OK (60 tests, 253 assertions)

@Fragonite
Copy link
Contributor Author

Hi @TomoTsuyuki,

Thank you for adding the unit tests. Approved.

@dmitriim
Copy link
Member

LGTM

@Fragonite Fragonite requested a review from jaypha August 28, 2024 14:38
@Fragonite
Copy link
Contributor Author

Hi @TomoTsuyuki and @jaypha,

I don't have permission to merge this. Are you able to help?

Cheers.

@dmitriim dmitriim self-requested a review August 28, 2024 21:56
@dmitriim dmitriim self-assigned this Aug 28, 2024
@dmitriim dmitriim merged commit ba37892 into main Aug 28, 2024
18 checks passed
@dmitriim dmitriim deleted the decode-content-main branch August 28, 2024 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmitriim dmitriim dmitriim approved these changes

@jaypha jaypha Awaiting requested review from jaypha

Assignees

@dmitriim dmitriim

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Fragonite @dmitriim @TomoTsuyuki